fix: reconcile pending bundles on a timer when blocks stall (SUP-1552) - #50
fix: reconcile pending bundles on a timer when blocks stall (SUP-1552)#50jayeshbhole wants to merge 5 commits into
Conversation
handleBlock is the only path that resolves a submitted bundle and returns its executor wallet to the sender pool, but watchBlocks only fires it on a new block. On a chain that produces blocks only when it receives a transaction, a bundler that stops submitting waits for a block that only it would have caused: no wallet is freed, every later bundle blocks forever in getWallet(), and userOps are accepted but never included. Keep the block-driven receipt polling (a receipt cannot change without a new block) and add a watchdog that runs handleBlock once no reconcile has happened within resubmitStuckTimeout while bundles are still pending. No RPC work on a healthy chain. Also hold currentlyHandlingBlock in try/finally: a throw in handleBlockInner left it set, wedging every later tick at the overlap guard.
|
|
||
| const pendingBundles = this.bundleManager.getPendingBundles().length | ||
| if (pendingBundles === 0) { | ||
| return |
There was a problem hiding this comment.
This branch leaves both watchBlocks and staleBlockTimer alive once the last pending bundle has been resolved on an idle chain: no later block calls handleBlockInner() to reach its existing cleanup. Please call this.stopWatchingBlocks() before returning.
| this.staleBlockTimer = setInterval(() => { | ||
| const msSinceReconcile = Date.now() - this.lastReconcileAt | ||
|
|
||
| if (msSinceReconcile < this.config.resubmitStuckTimeout) { |
There was a problem hiding this comment.
The watchdog fires when msSinceReconcile >= resubmitStuckTimeout, while potentiallyResubmitBundle() currently requires strict >. If this tick lands exactly on the boundary, the status check can no-op and reset lastReconcileAt, delaying a genuinely missing transaction for another timeout. Please align the existing isStuck check to >=.
|
|
||
| await vi.advanceTimersByTimeAsync(RESUBMIT_STUCK_TIMEOUT + BLOCK_TIME) | ||
|
|
||
| expect(getBundleStatuses).toHaveBeenCalled() |
There was a problem hiding this comment.
This assertion only proves that the watchdog invokes a status lookup. Because the mock returns [], it never exercises the production failure or recovery path. Please model not_found on the block tick followed by included on the watchdog tick, then assert that processIncludedBundle runs and the pending bundle/wallet is released; that is the regression we need to lock down.
SahilVasava
left a comment
There was a problem hiding this comment.
Requesting changes for the three inline findings: stop both watchers when no pending bundles remain, align the timeout boundary to >=, and add a regression test covering not_found -> watchdog -> included -> wallet released. The timer-based reconciliation approach is correct; these are the scoped changes needed before merge.
- stop watcher when watchdog finds no pending bundles - align isStuck to >= to match watchdog boundary - test not_found -> watchdog -> included -> bundle released
Temporary. Local e2e passes 3/3 on the review-fix commit while CI fails 2/2 on the same 5 parallel-op tests, so bisecting in CI.
Isolates which of the two review fixes breaks the parallel-op e2e tests.
Stopping watchBlocks when no bundles are pending failed 5 parallel-op e2e tests. Bisected in CI: fixes 1+2 red, neither green, fix 2 alone green. The idle-watcher leak predates this PR.
Part of SUP-1552. Closes ZER-928
Problem
Chain 55516 (Geo testnet) stalled for 19h45m on 2026-08-13/14.
eth_sendUserOperationkept returning 200 OK; no userOp ever got a receipt. The same pattern has recurred since June: 18d, 7.5d, 7d, 5.6d, 4.3d, plus a dozen 20–50h gaps.It is a deadlock, and the bundler is both halves of it.
handleBlockis the only path that resolves a submitted bundle: it reads receipts, appliesresubmitStuckTimeout, rotates stuck bundles, and returns the executor wallet to the sender pool. Its only trigger iswatchBlocks({ onBlock }), which fires on a new block.Arbitrum Orbit chains produce a block only when they receive a transaction. On a low-traffic testnet the bundler is effectively the only writer, so:
getWallet()is an unbounded spin (createRedisSenderManager.ts:80), so every later bundle parks there forever.resubmitStuckTimeoutis a time condition evaluated only on a block event, so the escape hatch is unreachable exactly when it is needed.Nothing inside the loop can break it. What actually broke it was the customer sending an unrelated transaction from outside; the relayer resumed one second after that block landed.
Evidence (BetterStack,
Ultra Relay (Prod), chainId 55516)eth_sendUserOperation(inbound)eth_getBlockByNumber(watcher poll)eth_sendRawTransaction(outbound)mempool-storelogged userOps goingoutstanding -> processingthroughout the stall, then nothing: they reachedsendBundleToExecutorand parked ingetWallet(). Zero warns or errors for 20h, which is the tell thathandleBlocknever executed once. The trigger was upstream rate limiting oneth_sendRawTransactionat 23:16–23:29 ({"code":-32017,"message":"Rate Limit Exceeded..."}).Change
1. Stale-block watchdog.
watchBlocksstays: receipt polling is correctly block-driven, since a receipt cannot change without a new block and each tick costs one receipt lookup per pending bundle plus gas price reads. Replacing it with an interval would have raised RPC load on the very endpoint that was rate-limiting us.Instead, a timer re-arms only the time-based check that the block gate makes unreachable. It calls
handleBlockwhen no reconcile has happened withinresubmitStuckTimeoutand bundles are still pending. On a healthy chain a block arrives everyblockTime, solastReconcileAtstays fresh and the watchdog does zero RPC. It fires astaleBlockWatchdogFiredwarn when it does act.This is the same fix
reconcileQuarantinedWalletsalready applies one layer down, for the same reason: that code path's comment notes that waiting onlatest > stuckNoncewould deadlock, because the benched wallet sends nothing. Same bug, one level up. No new config option;resubmitStuckTimeoutandblockTimealready exist.2.
currentlyHandlingBlockheld intry/finally. It was set at the top ofhandleBlockInnerand cleared only on the success path. A throw in there (a store error infreeSubmittedBundle, say) left it set, so every later tick returned early at the overlap guard and reconciliation never resumed for the life of the process. That would also have silently defeated the watchdog above, so it is fixed here rather than separately.Tests
src/executor/executorManager.test.ts, 4 cases with fake timers. Each was confirmed to fail with the corresponding fix reverted:try/finallytry/finallypnpm lint: 0 errors (the 902 warnings are pre-existing onmain).Not in this PR
-32017was still firing on Aug 25 (02:36, 02:44, 03:21, 06:45, 08:01). This PR stops it from causing a multi-day outage; it does not stop the sends from failing. Needs an API key on the deployment.getWallet()timeout. The unbounded spin atcreateRedisSenderManager.ts:80is a landmine regardless. Worth bounding as defense-in-depth, but it treats starvation rather than the cause, so it is left out here.